Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent double callback in _executeRequest #363

Merged
merged 1 commit into from
Jul 22, 2022

Conversation

episage
Copy link
Contributor

@episage episage commented Nov 16, 2021

www.googleapis.com does ECONNRESET just after data is received in passBackControl

This prevents the callback from being called twice.

@8bitDesigner
Copy link

I just lost two days of work troubleshooting this bug. I hate to be the 👍 guy, but it would be great to get this merged.

@shayneczyzewski
Copy link

Hitting this issue (just on server startup and first request) myself. Any ides on if/when this may be merged? Thanks!

@MithileshHinge
Copy link

+1 Lost 2 working days right before demo day because of this issue. Please merge this PR, thank you.

@konotorii
Copy link

+1 behind schedule with no way of fixing it, would be greatly appreciated if merged, thanks.

@episage
Copy link
Contributor Author

episage commented Jul 22, 2022

Guys, this is open source software. Nobody gets paid to merge this.
If you want to use the fix just install from my branch:

npm install https://github.com/episage/node-oauth.git#db8088793f6a6ce27967068d661c9810cfc076e0

I promise not to delete this code in the next 100 years.

@ciaranj
Copy link
Owner

ciaranj commented Jul 22, 2022 via email

@shayneczyzewski
Copy link

shayneczyzewski commented Jul 22, 2022

Thank you very much @ciaranj, we really appreciate it!

The problem is a bit more nuanced than that, @episage. I fully agree the burden of package managers is unpaid, thankless and unending. So again, thank you Ciaran! However, this package is a 3rd level dependency for the Google Passport package (and probably others). Therefore, it is not as simple as "fixing it yourself". You have to fork and update the package dependencies for 2 other projects as well, and then stay in line with those as they evolve over time.

One option in the meantime for others on this list, which we are using right now, is https://www.npmjs.com/package/patch-package (which I found this PR and that solution referenced here: jaredhanson/passport-google-oauth2#87)

EDIT: And I should have also said thank you, Tom, as well, for this fix! :D

@ciaranj ciaranj merged commit be56ccc into ciaranj:master Jul 22, 2022
@episage
Copy link
Contributor Author

episage commented Jul 22, 2022

xD This was fast. Nice one!

@ciaranj
Copy link
Owner

ciaranj commented Jul 22, 2022

I've just published node-oauth 0.10.0 with this fix in it. I chose to bump the minor version rather than patch as it may affect calling code.

@8bitDesigner
Copy link

@ciaranj, you are a golden god. Thank you!

@konotorii
Copy link

👏👏 absolute life saver

@Rishabh570
Copy link

This is amazing, thank you @ciaranj 🙌

@DevinCLane
Copy link

you saved my life

gabegorelick added a commit to gabegorelick/passport-oauth1 that referenced this pull request Oct 18, 2024
Mostly for ciaranj/node-oauth#363, which fixes
jaredhanson/passport-oauth2#163. I haven't been able to reproduce that
issue with passport-twitter, but in theory it should be possible.

I'm not aware of any breaking changes in `oauth`.

See https://github.com/ciaranj/node-oauth/commits
gabegorelick added a commit to gabegorelick/passport-oauth1 that referenced this pull request Oct 18, 2024
Mostly for ciaranj/node-oauth#363, which fixes
jaredhanson/passport-oauth2#163. I haven't been able to reproduce that
issue with passport-oauth1, but in theory it may be possible.

I'm not aware of any breaking changes in `oauth`.

See https://github.com/ciaranj/node-oauth/commits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants